Skip to content

feat: guard Deployer singleton before Host construction#4189

Open
m1rm wants to merge 1 commit intodeployphp:masterfrom
m1rm:feat/deployer-has-instance
Open

feat: guard Deployer singleton before Host construction#4189
m1rm wants to merge 1 commit intodeployphp:masterfrom
m1rm:feat/deployer-has-instance

Conversation

@m1rm
Copy link
Copy Markdown

@m1rm m1rm commented Apr 11, 2026

  • Bug fix #…?
  • New feature?
  • BC breaks? -> maybe, because the exception changed
  • Tests added?
  • Docs added? -> comments in the code

Background

Host::__construct used Deployer::get() to decide whether to inherit the global recipe config. With a typed, uninitialized static $instance, PHP 8+ throws when get() reads the property before new Deployer(...) has run. That can break HostTest (and any code path that builds a Host without a bootstrapped singleton).

Solution

Deployer:

  • Store the singleton as ?self (default null). get() throws a clear RuntimeException if not initialized.
  • Add hasInstance() so callers can check without touching an uninitialized property.
  • Add resetInstance() (@internal) for tests that need a clean singleton between cases.

Host:

  • Use Deployer::hasInstance() before calling get() when wiring the parent Configuration.

HostTest:

  • tearDown() calls Deployer::resetInstance() so tests stay isolated.
  • Import Deployer\Deployer for tearDown.

Behavior / compatibility

  • Normal runtime (always new Deployer before heavy use): unchanged.
  • Calling Deployer::get() with no instance: still invalid; error is now RuntimeException instead of the engine error on an uninitialized typed static.

@m1rm
Copy link
Copy Markdown
Author

m1rm commented Apr 11, 2026

Additional info/motivation of this PR: I came across the issue when implementing setDefaultSelector as thin typed wrapper for set('default_selector', ...) (which will be a subsequent PR).

$parent = null;
if (Deployer::get()) {
if (Deployer::hasInstance()) {
$parent = Deployer::get()->config;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a use case where we create a Host instance before creating Deployer?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should throw an error here instead?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a use case where we create a Host instance before creating Deployer?

I ran into an error when trying to implement a thin typed wrapper for set('default_selector', ...), the wip implementation is here:

master...m1rm:deployer:feat/add-setDefaultSelector-function

maybe I also took a wrong turn, I'd be fine with throwing an error if that is more appropriate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants